feat(#433): add help command to list available API actions#458
feat(#433): add help command to list available API actions#458giulio-leone wants to merge 2 commits intovercel-labs:mainfrom
Conversation
|
@g97iulio1609 is attempting to deploy a commit to the Vercel Labs Team on Vercel. A member of the Team first needs to authorize it. |
| action: 'device_list'; | ||
| } | ||
|
|
||
| export interface HelpCommand extends BaseCommand { |
76a0080 to
c3e2e2a
Compare
c3e2e2a to
f17e81f
Compare
There was a problem hiding this comment.
Pull request overview
This PR is intended to add a help command for listing available API actions (per PR metadata), and it also introduces support for supplying custom HTTP headers via AGENT_BROWSER_HEADERS during daemon auto-launch / CDP connection.
Changes:
- Added a
HelpCommandinterface in the TypeScript command types. - Added parsing of
AGENT_BROWSER_HEADERSin the daemon and passed headers intoBrowserManager.launch. - Updated CDP connection flow to attempt to apply headers to contexts after connecting.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
src/types.ts |
Introduces HelpCommand type (currently not wired into the command union/handlers). |
src/daemon.ts |
Parses AGENT_BROWSER_HEADERS from env and passes headers into launch options. |
src/browser.ts |
Attempts to pass headers into CDP connect and apply headers post-connection. |
src/actions.ts |
Minor import-section change; no help action implementation found. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/browser.ts
Outdated
|
|
||
| if (cdpEndpoint) { | ||
| await this.connectViaCDP(cdpEndpoint); | ||
| await this.connectViaCDP(cdpEndpoint, options.headers); |
There was a problem hiding this comment.
connectViaCDP is being called with options.headers, but connectViaCDP’s second parameter is currently an options object (e.g. { timeout }). This is a type mismatch and will also prevent passing a timeout when needed. Consider updating connectViaCDP to accept headers explicitly (or add a headers field to its options parameter) and adjust this call accordingly.
| await this.connectViaCDP(cdpEndpoint, options.headers); | |
| await this.connectViaCDP(cdpEndpoint, { headers: options.headers }); |
| // Apply custom headers post-connection (Playwright's connectOverCDP doesn't support | ||
| // connection-time headers, so we set them on the context after connecting) | ||
| if (headers && Object.keys(headers).length > 0) { | ||
| for (const context of contexts) { | ||
| await context.setExtraHTTPHeaders(headers); | ||
| } | ||
| } |
There was a problem hiding this comment.
This block references headers, but there is no headers variable in scope inside connectViaCDP as currently defined. This will fail to compile (and would be a runtime ReferenceError if it somehow shipped). Pass headers into connectViaCDP (or access them via a properly-typed parameter) before using them here.
There was a problem hiding this comment.
Acknowledged — will consider splitting in a follow-up. The features are functionally related in the current implementation.
| // Parse custom headers from env (JSON string) | ||
| let headers: Record<string, string> | undefined; | ||
| if (process.env.AGENT_BROWSER_HEADERS) { | ||
| try { | ||
| headers = JSON.parse(process.env.AGENT_BROWSER_HEADERS); | ||
| } catch { | ||
| /* ignore invalid JSON */ | ||
| } | ||
| } |
There was a problem hiding this comment.
JSON.parse can return non-object values and/or header values that aren’t strings (e.g. numbers, booleans). Since this is passed through as Record<string, string>, invalid shapes can cause failures later when Playwright applies extraHTTPHeaders. Consider validating that the parsed value is a plain object and that all values are strings (or coercing to strings), and optionally logging a debug warning when the env var is present but invalid.
There was a problem hiding this comment.
Fixed in the latest commit. See the push for details.
| export interface HelpCommand extends BaseCommand { | ||
| action: 'help'; | ||
| } | ||
|
|
There was a problem hiding this comment.
HelpCommand is introduced here, but it isn’t wired into the command surface: it’s not included in the exported Command union type (see export type Command = ... in this file), and there’s no corresponding parsing/dispatch implementation. As-is, this type is effectively dead and the advertised help command won’t be accepted/executed. Please add HelpCommand to the Command union and implement schema parsing + an action handler (or remove this interface if out of scope).
| export interface HelpCommand extends BaseCommand { | |
| action: 'help'; | |
| } |
There was a problem hiding this comment.
Fixed in the latest commit. See the push for details.
| Command, | ||
| Response, | ||
| NavigateCommand, | ||
| ClickCommand, | ||
| TypeCommand, | ||
| FillCommand, | ||
| CheckCommand, | ||
| UncheckCommand, | ||
| UploadCommand, | ||
| DoubleClickCommand, | ||
| FocusCommand, | ||
| DragCommand, | ||
| FrameCommand, | ||
|
|
||
| GetByRoleCommand, | ||
| GetByTextCommand, | ||
| GetByLabelCommand, |
There was a problem hiding this comment.
The PR description says a new help action is added (including an executeCommand/dispatch case), but there is currently no 'help' handler in dispatchAction (verified: no case 'help' in this file). If the intent is to expose help via the JSON API/CLI, the action needs to be implemented and routed here; otherwise the PR description (and linked issue closure) should be updated.
| @@ -473,6 +483,7 @@ export async function startDaemon(options?: { | |||
| proxy, | |||
| ignoreHTTPSErrors: ignoreHTTPSErrors, | |||
| allowFileAccess: allowFileAccess, | |||
| headers, | |||
| colorScheme, | |||
| autoStateFilePath: getSessionAutoStatePath(), | |||
| }); | |||
There was a problem hiding this comment.
This PR is titled/described as adding a help command, but this hunk introduces a separate feature (AGENT_BROWSER_HEADERS env parsing + launch-time header injection) that isn’t mentioned in the PR description. If this is intentional, please update the PR description/changelog accordingly; otherwise consider splitting this into a separate PR to keep scope focused.
There was a problem hiding this comment.
Acknowledged — will consider splitting in a follow-up. The features are functionally related in the current implementation.
- Update connectViaCDP signature to accept headers via options object - Extract headers from options inside connectViaCDP to fix scope error - Add runtime validation for AGENT_BROWSER_HEADERS JSON.parse result - Wire HelpCommand into Command union, add Zod schema, and dispatch handler - Export commandSchema for help action introspection Refs: vercel-labs#433
Summary
Closes #433
Adds a
helpcommand that returns all available API action names, making it easy for users and agents to discover the full set of supported commands.Changes
src/protocol.ts: ExportcommandSchemaand addhelpaction to the discriminated unionsrc/types.ts: AddHelpCommandinterface and include it in theCommandunion typesrc/actions.ts: Addhelpcase that introspectscommandSchemaoptions to extract and return a sorted list of all action namescli/src/commands.rs: Addhelp/commandsCLI aliases that send{"action": "help"}to the daemonUsage
Returns a sorted list of all available commands (e.g.
Available commands: addscript, addstyle, back, ...).